Skip to content

Implement search-by-category for Import/Export wizard #2602

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Dec 9, 2024
Merged

Implement search-by-category for Import/Export wizard #2602

merged 1 commit into from
Dec 9, 2024

Conversation

@ptziegler
Copy link
Contributor

@ptziegler ptziegler commented Dec 9, 2024

This adapts the WizardPatternFilter to also show the wizards, if the search string matches one of its containing categories.

Amends 532c0fb
Fixes #2576

@ptziegler
Implement search-by-category for Import/Export wizard
50a19d6 
This adapts the WizardPatternFilter to also show the wizards, if the
search string matches one of its containing categories.

Amends 532c0fb
Fixes eclipse-platform#2576
@ptziegler ptziegler mentioned this pull request Dec 9, 2024
Closed
@vogella
Copy link
Contributor

vogella commented Dec 9, 2024

LGTM, tested it and works perfectly.

@ptziegler
Copy link
Contributor Author

ptziegler commented Dec 9, 2024

Example:

image

Note that there is a special implementation for the content provider which flattens categories with only a single child:

image

This behavior is intentional, but whether it's a "good" behavior is a different question...

eclipse.platform.ui/bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/internal/dialogs/WizardContentProvider.java

Lines 49 to 52 in 1876695

// flatten lists with only one category
if (list.size() == 1 && list.get(0) instanceof WizardCollectionElement) {
return getChildren(list.get(0));
}

@vogella
Copy link
Contributor

vogella commented Dec 9, 2024

This behavior is intentional, but whether it's a "good" behavior is a different question...

So you suggest to remove this flattening?

@ptziegler
Copy link
Contributor Author

ptziegler commented Dec 9, 2024

So you suggest to remove this flattening?

That would be my preference because the current behavior is not very intuitive. A WizardCollectionElement is flattened, if it doesn't have any other siblings. So the visibility of an element depends on your search string.

image

I found it confusion when Test Import Add To Parent Category was no longer a direct child of General after clearing the filter, even though that's exactly what the viewer told me.

However, this has been the behavior for over 20 years without any complaints. So I don't think the current behavior is wrong and if it is changed, then only as part of a separate commit.

@vogella
Copy link
Contributor

vogella commented Dec 9, 2024

That would be my preference because the current behavior is not very intuitive. A WizardCollectionElement is flattened, if it doesn't have any other siblings. So the visibility of an element depends on your search string.

I agree that we should remove this special treatment, I think consistent behavior is the UI is important. As you already into the code, please provide a separate PR for this adjustment.

@vogella
Copy link
Contributor

vogella commented Dec 9, 2024

Linux builds seem to fail always unrelated to the actual change so I will assume that this is not related to this particular change.

org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.eclipse.tycho:tycho-apitools-plugin:4.0.10:verify (verify) on project org.eclipse.ui.views.properties.tabbed: Execute ApiApplication failed

@vogella
Copy link
Contributor

vogella commented Dec 9, 2024

Linux build issue reported here: #2603

@github-actions
Copy link
Contributor

github-actions bot commented Dec 9, 2024

Test Results

 1 214 files   -   306   1 214 suites   - 306   1h 21m 39s ⏱️ - 12m 0s
 7 729 tests ±    0   7 495 ✅  -     6  233 💤 + 5  1 ❌ +1 
16 232 runs   - 6 456  15 716 ✅  - 6 419  515 💤  - 38  1 ❌ +1 

For more details on these failures, see this check.

Results for commit 50a19d6. ± Comparison against base commit 1876695.

This pull request skips 5 tests. 
org.eclipse.ui.genericeditor.tests.HoverTest ‑ testEnabledWhenHover
org.eclipse.ui.genericeditor.tests.HoverTest ‑ testMultipleHover
org.eclipse.ui.genericeditor.tests.HoverTest ‑ testProblemHover
org.eclipse.ui.genericeditor.tests.HoverTest ‑ testSingleHover
org.eclipse.ui.genericeditor.tests.ShowInformationTest ‑ testInformationControl

ptziegler added a commit to ptziegler/eclipse.platform.ui that referenced this pull request Dec 9, 2024
@ptziegler
Remove special flattening-behavior for Wizard content providers
20eac43 
This removes the feature from the "New" and "Import/Export" wizards,
where container for wizard elements would be flattened, if it only
contains a single, other container.

There are two reasons for this change:
- it is not consistent with the other viewers, that always show the
complete tree path (e.g. the preference dialog).
- it doesn't work properly, as it is still possible to produce the
undesirable behavior in the "New" wizard (for e.g. Java/JUnit/...)

See eclipse-platform#2602
for context.
@ptziegler ptziegler mentioned this pull request Dec 9, 2024
Merged
ptziegler added a commit to ptziegler/eclipse.platform.ui that referenced this pull request Dec 9, 2024
@ptziegler
Remove special flattening-behavior for Wizard content providers
5d0d4fa 
This removes the feature from the "New" and "Import/Export" wizards,
where container for wizard elements would be flattened, if it only
contains a single, other container.

There are two reasons for this change:
- it is not consistent with the other viewers, that always show the
complete tree path (e.g. the preference dialog).
- it doesn't work properly, as it is still possible to produce the
undesirable behavior in the "New" wizard (for e.g. Java/JUnit/...)

See eclipse-platform#2602
for context.
@ptziegler
Copy link
Contributor Author

ptziegler commented Dec 9, 2024

Linux builds seem to fail always unrelated to the actual change so I will assume that this is not related to this particular change.

org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.eclipse.tycho:tycho-apitools-plugin:4.0.10:verify (verify) on project org.eclipse.ui.views.properties.tabbed: Execute ApiApplication failed

The builds really are flaky... 😦

As you already into the code, please provide a separate PR for this adjustment.

#2604

@vogella
Copy link
Contributor

vogella commented Dec 9, 2024

The search test failure on Mac also seems unrelated, I checked them and I do not see the Wizard involved. Merging this one.

@vogella vogella merged commit e69ed28 into eclipse-platform:master Dec 9, 2024
14 of 17 checks passed
vogella pushed a commit that referenced this pull request Dec 10, 2024
@ptziegler @vogella
Remove special flattening-behavior for Wizard content providers
d58c50e 
This removes the feature from the "New" and "Import/Export" wizards,
where container for wizard elements would be flattened, if it only
contains a single, other container.

There are two reasons for this change:
- it is not consistent with the other viewers, that always show the
complete tree path (e.g. the preference dialog).
- it doesn't work properly, as it is still possible to produce the
undesirable behavior in the "New" wizard (for e.g. Java/JUnit/...)

See #2602
for context.
@vogella
Copy link
Contributor

vogella commented Dec 10, 2024

@ptziegler thanks, works great in todays I-Build. I think this is a notable feature, can you add it to N&N 4.5? I think the repo for this is: https://github.com/eclipse-platform/www.eclipse.org-eclipse?tab=readme-ov-file

@ptziegler ptziegler deleted the wizard-filter branch January 14, 2025 18:57
ptziegler added a commit to ptziegler/www.eclipse.org-eclipse that referenced this pull request Jan 21, 2025
@ptziegler
Add news & noteworthy entry regarding "Search by Category"
37fa9ab 
The "Show View" as well as the "Import" and "Export" wizards now support
searching by category, rather than only by entry.

eclipse-platform/eclipse.platform.ui#2576
eclipse-platform/eclipse.platform.ui#2579
eclipse-platform/eclipse.platform.ui#2602
ptziegler added a commit to ptziegler/www.eclipse.org-eclipse that referenced this pull request Jan 21, 2025
@ptziegler
Add news & noteworthy entry regarding "Filter by Category"
aab6e5e 
The "Show View" as well as the "Import" and "Export" wizards now support
filtering by category, rather than only by entry.

eclipse-platform/eclipse.platform.ui#2576
eclipse-platform/eclipse.platform.ui#2579
eclipse-platform/eclipse.platform.ui#2602
@ptziegler ptziegler mentioned this pull request Jan 21, 2025
Merged
vogella pushed a commit to eclipse-platform/www.eclipse.org-eclipse that referenced this pull request Jan 21, 2025
@ptziegler @vogella
Add news & noteworthy entry regarding "Filter by Category"
537b34a 
The "Show View" as well as the "Import" and "Export" wizards now support
filtering by category, rather than only by entry.

eclipse-platform/eclipse.platform.ui#2576
eclipse-platform/eclipse.platform.ui#2579
eclipse-platform/eclipse.platform.ui#2602
ptziegler added a commit to ptziegler/www.eclipse.org-eclipse that referenced this pull request Jan 21, 2025
@ptziegler
Add news & noteworthy entry regarding "Filter by Category"
ce13739 
The "Show View" as well as the "Import" and "Export" wizards now support
filtering by category, rather than only by entry.

eclipse-platform/eclipse.platform.ui#2576
eclipse-platform/eclipse.platform.ui#2579
eclipse-platform/eclipse.platform.ui#2602
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

New Wizard Dialog should also include Categories into Filter

2 participants

@ptziegler @vogella